Skip to content

Conversation

@ws4charlie
Copy link
Contributor

@ws4charlie ws4charlie commented Sep 26, 2025

Description

The go-tss side modification is in PR 64

Closes #1038

How Has This Been Tested?

  • Tested CCTX in localnet
  • Tested in development environment
  • Go unit tests
  • Go integration tests
  • Tested via GitHub Actions

Summary by CodeRabbit

  • New Features
    • Added support for configuring a public DNS name for zetaclient.
    • New CLI flag: --public-dns; config now includes PublicDNS.
    • Telemetry now accepts DNS name and exposes it via a /dns endpoint.
    • TSS supports DNS-based external addressing (works with either public IP or DNS).
    • Localnet: hostname-to-IP mappings added; init script prefers DNS over IP.
  • Documentation
    • Changelog updated to note DNS support.
  • Chores
    • Upgraded go-tss dependency to a newer pre-release version.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Sep 26, 2025

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

📝 Walkthrough

Walkthrough

Adds optional public DNS support alongside public IP throughout zetaclient: CLI flag and config field, telemetry server DNS handling and endpoint, TSS setup accepting DNS, localnet hostname mappings and startup script changes, and bumps go-tss to a pre-release with hostname support. Changelog updated accordingly.

Changes

Cohort / File(s) Summary
Docs
changelog.md
Notes new feature: support for zetaclient public DNS name.
CLI and Config Plumbed Public DNS
cmd/zetaclientd/initconfig.go, zetaclient/config/types.go
Adds --public-dns flag, stores value in initializeConfigOptions, assigns to configData.PublicDNS; adds exported Config.PublicDNS (json:"PublicDNS"). Minor operator flag text tweak.
Telemetry DNS Support
cmd/zetaclientd/start.go, zetaclient/metrics/telemetry.go
TelemetryServer gains dnsName field, SetDNSName/GetDNSName, and /dns endpoint; start flow sets telemetry DNS from cfg.PublicDNS.
TSS Hostname Enablement
zetaclient/tss/setup.go, go.mod
Requires either PublicIP or PublicDNS; passes ExternalDNS from PublicDNS to TSS server; updates go-tss to v0.6.4-0.20250924153151-2ae33848328f.
Localnet Hostname Wiring
contrib/localnet/docker-compose.yml, contrib/localnet/scripts/start-zetaclientd.sh
Adds extra_hosts mapping zetaclientN.com -> container IPs; startup script switches non-zetacore0 init from --public-ip to --public-dns and drops explicit log level.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant User as Operator
  participant CLI as zetaclientd (init)
  participant FS as Config File
  participant Svc as zetaclientd (start)
  participant Tele as TelemetryServer

  User->>CLI: zetaclientd init --public-dns=<host>
  CLI->>FS: write Config{ PublicDNS: <host>, ... }

  User->>Svc: zetaclientd start
  Svc->>FS: read Config
  Svc->>Tele: NewTelemetryServer()
  Svc->>Tele: SetIPAddress(cfg.PublicIP)
  Svc->>Tele: SetDNSName(cfg.PublicDNS)
  Tele-->>User: /dns HTTP returns cfg.PublicDNS
Loading
sequenceDiagram
  autonumber
  participant Svc as zetaclientd (start)
  participant TSS as go-tss
  note over Svc: Validate endpoints
  Svc->>Svc: if cfg.PublicIP=="" && cfg.PublicDNS=="" then warn
  Svc->>TSS: CreateServer(ExternalIP=cfg.PublicIP,<br/>ExternalDNS=cfg.PublicDNS, ...)
  TSS-->>Svc: Server initialized (IP and/or DNS-based)
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Pre-merge checks and finishing touches

❌ Failed checks (3 warnings)
Check name Status Explanation Resolution
Out of Scope Changes Check ⚠️ Warning The pull request introduces changes to the telemetry server HTTP route, updates localnet Docker Compose extra_hosts entries, and modifies the start-zetaclientd.sh script for testing environments, all of which are not directly required to address the hostname-based TSS networking support outlined in issue #1038. Please consider moving the telemetry endpoint additions and localnet environment adjustments into a separate PR or clearly documenting them as ancillary test enhancements.
Description Check ⚠️ Warning The pull request description correctly references the go-tss dependency and provides a testing checklist, but it does not include a clear summary of the changes introduced in this repository or the motivation and context for those modifications. Please expand the Description section to include a concise summary of the code changes, the rationale for adding external DNS support, and any dependencies such as the referenced go-tss PR.
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Title Check ✅ Passed The title succinctly summarizes the core change—adding support for an external DNS name in the zetaclient CLI—and adheres to conventional formatting with a clear feature marker.
Linked Issues Check ✅ Passed The implementation adds a new CLI flag and configuration field for public DNS, maps the value into the Zeta client and TSS setup, and guards network configuration to allow a hostname in addition to an IP address, fulfilling the hostname support requested in issue #1038.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@codecov
Copy link

codecov bot commented Sep 26, 2025

Codecov Report

❌ Patch coverage is 34.28571% with 23 lines in your changes missing coverage. Please review.
✅ Project coverage is 65.77%. Comparing base (099d3ac) to head (d892b7f).
⚠️ Report is 1 commits behind head on develop.

Files with missing lines Patch % Lines
zetaclient/metrics/telemetry.go 0.00% 12 Missing ⚠️
zetaclient/config/config.go 60.00% 8 Missing ⚠️
zetaclient/tss/setup.go 0.00% 3 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #4254      +/-   ##
===========================================
- Coverage    65.79%   65.77%   -0.03%     
===========================================
  Files          466      466              
  Lines        34386    34416      +30     
===========================================
+ Hits         22625    22637      +12     
- Misses       10764    10782      +18     
  Partials       997      997              
Files with missing lines Coverage Δ
zetaclient/config/types.go 48.93% <ø> (ø)
zetaclient/tss/setup.go 0.00% <0.00%> (ø)
zetaclient/config/config.go 24.71% <60.00%> (+4.84%) ⬆️
zetaclient/metrics/telemetry.go 0.00% <0.00%> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@ws4charlie ws4charlie marked this pull request as ready for review September 26, 2025 15:56
@ws4charlie ws4charlie requested a review from a team as a code owner September 26, 2025 15:56
@ws4charlie ws4charlie requested a review from a team September 26, 2025 15:56
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

🧹 Nitpick comments (1)
contrib/localnet/scripts/start-zetaclientd.sh (1)

107-112: Ensure DNS mapping exists for all peers; consider parameterizing DNS suffix.

Using --public-dns "$HOSTNAME.com" assumes all containers can resolve every peer’s “.com” FQDN. Please add extra_hosts for all peers (including zetaclient-new-validator) across all zetaclient services in docker-compose, not just self-mappings. Also consider a DNS suffix env (e.g., DNS_SUFFIX, default ".com") to avoid hard-coding.

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 76d8958 and a503537.

⛔ Files ignored due to path filters (1)
  • go.sum is excluded by !**/*.sum
📒 Files selected for processing (9)
  • changelog.md (1 hunks)
  • cmd/zetaclientd/initconfig.go (3 hunks)
  • cmd/zetaclientd/start.go (1 hunks)
  • contrib/localnet/docker-compose.yml (4 hunks)
  • contrib/localnet/scripts/start-zetaclientd.sh (1 hunks)
  • go.mod (1 hunks)
  • zetaclient/config/types.go (1 hunks)
  • zetaclient/metrics/telemetry.go (4 hunks)
  • zetaclient/tss/setup.go (2 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.go

⚙️ CodeRabbit configuration file

Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.

Files:

  • cmd/zetaclientd/start.go
  • zetaclient/config/types.go
  • zetaclient/metrics/telemetry.go
  • zetaclient/tss/setup.go
  • cmd/zetaclientd/initconfig.go
**/*.sh

⚙️ CodeRabbit configuration file

Review the shell scripts, point out issues relative to security, performance, and maintainability.

Files:

  • contrib/localnet/scripts/start-zetaclientd.sh
🧠 Learnings (2)
📚 Learning: 2025-09-21T17:54:21.110Z
Learnt from: kingpinXD
PR: zeta-chain/node#4225
File: Makefile:383-385
Timestamp: 2025-09-21T17:54:21.110Z
Learning: In upgrade tests, docker-compose-upgrade.yml is used specifically to build older versions of zetacored and zetaclientd, while docker-compose.yml handles the newer version configuration. Environment variables like USE_ZETAE2E_ANTE are intended for the newer version and don't need to be propagated to the upgrade compose file.

Applied to files:

  • contrib/localnet/docker-compose.yml
📚 Learning: 2024-09-24T18:43:46.232Z
Learnt from: gartnera
PR: zeta-chain/node#2817
File: contrib/rpcimportable/go.mod:1-3
Timestamp: 2024-09-24T18:43:46.232Z
Learning: In this project, minor version differences in Go modules are acceptable and do not require strict consistency.

Applied to files:

  • go.mod
🔇 Additional comments (8)
cmd/zetaclientd/start.go (1)

227-228: LGTM: Telemetry sets DNS name from config.

Setting DNS alongside IP keeps telemetry consistent with the new PublicDNS field.

zetaclient/config/types.go (1)

82-82: LGTM: Config surface extended with PublicDNS.

Field name and JSON tag are consistent with existing style. No masking required.

cmd/zetaclientd/initconfig.go (2)

15-18: LGTM: Adds publicDNS option to CLI struct.

Matches the new config field; no concerns.


50-57: LGTM: Adds --public-dns flag.

Clear help text and parity with --public-ip.

go.mod (1)

309-309: Confirm ExternalDNS support in go-tss pseudo-version and update go.sum
Verify that commit 2ae33848328f in github.com/zeta-chain/go-tss introduces NetworkConfig.ExternalDNS and related DNS handling, then run go mod tidy and commit the updated go.sum.

zetaclient/metrics/telemetry.go (3)

24-37: dnsName field integrates cleanly.

The new dnsName state slots in alongside the existing telemetry attributes without upsetting synchronization or construction flows.


111-123: Getter/setter concurrency handling is consistent.

Locking mirrors the other accessors, so the DNS name stays thread-safe with minimal surface area.


198-255: Telemetry DNS endpoint is wired correctly.

The /dns route follows the existing pattern (status code first, plain response body) and keeps the handler focused and maintainable.

@renan061
Copy link
Contributor

Are the IP and the DNS mutually exclusive? Is it a XOR or an OR?
We could have that in the docs/hints/descriptions.

@ws4charlie ws4charlie requested a review from lumtis October 20, 2025 15:07
@ws4charlie ws4charlie enabled auto-merge October 21, 2025 15:02
@ws4charlie ws4charlie added this pull request to the merge queue Oct 21, 2025
Merged via the queue into develop with commit fb0d36f Oct 21, 2025
48 of 49 checks passed
@ws4charlie ws4charlie deleted the feat-support-external-DNS branch October 21, 2025 15:38
lumtis pushed a commit that referenced this pull request Oct 29, 2025
* add support for zetaclient external DNS name

* add changelog entry

* point to go-tss tag; fix changelog PR # and add zetaclient config description

* add zetaclient config file validation method
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support hostname in addition to public IP for zetaclient

5 participants